-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EAH - Typescript state_monitor_factory #23945
EAH - Typescript state_monitor_factory #23945
Conversation
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
Failed on:
jenkins, test this |
💔 Build Failed |
Cleaning up my review queue, please re-request review whenever you need it and CI is green (looks good so far though). |
910fa1c
to
cb4278c
Compare
💔 Build Failed |
Failed on the known APM Monitoring test failure, which is being fixed. Will rebase and rerun when it is merged - #24085 |
cb4278c
to
4698b87
Compare
💔 Build Failed |
4698b87
to
09db49a
Compare
💔 Build Failed |
Failed on: jenkins, test this |
💚 Build Succeeded |
Green! Good for another review @azasypkin, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -64,7 +64,7 @@ export class EmbeddedVisualizeHandler { | |||
}, 100); | |||
|
|||
private dataLoaderParams: RequestHandlerParams; | |||
private appState: AppState; | |||
private appState?: AppState; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can be marked as readonly
as it's only set in constructor.
81e5d19
to
26b797c
Compare
💚 Build Succeeded |
* Typescript state_monitor_factory * Fix linter error with possibly undefined * Expand typings to include hash stuff and expand the State type definition more. * Mark readonly
Typescript state_monitor_factory